Fix rewriting of configured async enumeration and disposal by frankshearar · Pull Request #527 · microsoft/coyote

Frank Shearar (frankshearar) · 2026-10-22T18:43:13Z

Summary

Fix a process-crashing rewrite of configured async enumeration on .NET 8, including
await foreach (var item in source.WithCancellation(token)) and
source.ConfigureAwait(...). Also handle configured async disposal, which shares
the same configured-ValueTask producer boundary.

Rebased onto upstream main. The constructor prerequisite, #529,
merged on October 23, 2026 and is no longer part of this PR's diff. All integration
regressions previously added in the merge commit are preserved.

The rewriter already replaces ConfiguredValueTaskAwaitable and its awaiters,
but did not replace the framework's configured enumerable, enumerator, or
disposable. Their methods therefore produced native structs for consumers
rewritten to expect Coyote's different struct layout.

Changes

Map TaskAsyncEnumerableExtensions, ConfiguredCancelableAsyncEnumerable<T>,
its nested enumerator, and ConfiguredAsyncDisposable to Coyote wrappers.
Route MoveNextAsync and DisposeAsync through the existing controllable
configured-ValueTask awaitables.
Preserve cancellation-token replacement and configuration chaining. Existing
awaiter behavior continues to force context capture during controlled testing.
Rewrite static and bound ldftn/ldvirtftn method-group targets along with
their delegate types, preserving instance binding instead of using static adapters.
Visit all bodyless signatures in the member pass, including custom delegates,
abstract methods, and interface methods, rather than skipping them with
instruction-pass traversal.
Rewrite type operands consistently, including boxing/unboxing and array
allocation, stores, loads, and element addresses. Preserve rewritten caller
type arguments when invoking external generic methods.
Rewrite base-type and implemented-interface references consistently with
method signatures. Keep base-constructor calls as instance calls when their
generic arguments are remapped.
Rewrite closed-generic explicit-interface override declarations and both
method and type generic constraints. Preserve instance call opcodes, including
nonvirtual base dispatch and callvirt on nonvirtual instance getters.
Rewrite field-reference declaring types alongside field types for all field
operands, including instance/static loads, stores, and addresses.
Rewrite synthetic rectangular-array constructors and accessors without
resolving nonexistent method definitions. Remap caller-supplied arguments on
external generic declaring types as well as generic methods, preserving
external concrete signatures and instance/base-constructor binding.
Rewrite method and constructor ldtoken operands while preserving their opcode
and instance binding, including expression-tree construction and compilation.
Remap property, indexer and event metadata alongside accessor signatures.
Keep generic method tokens consistent with their paired type handles by
preserving native declaring types for static-adapter-only replacements.
Forward ToBlockingEnumerable<T> to its native implementation under
NET7_0_OR_GREATER, preserving mapped generic arguments and cancellation.
Include 53 configured regression/control cases covering synchronous/asynchronous
iteration and disposal, context-capture settings, cancellation, chained
configuration, early-exit cleanup, failure replay, generic constructors
(including the nested enumerator), generic/custom enumeration delegates, and
non-generic disposal delegates, bound and virtual method groups, boxed round
trips, arrays of all three configured structs, an instance-adapter control,
generic inheritance, abstract/explicit-interface parameter and return types,
closed-generic explicit implementations, virtual base calls, generic constraints,
tuple fields, generic static fields, native null-source exception parity,
rectangular arrays, unrewritten generic containers and base classes,
method/constructor expression tokens, native instance-method binding, and
blocking enumeration of configured structs and previously mapped awaiters,
property/indexer expressions, event metadata/dispatch, and native generic
static-member expression handles with both native and mapped type arguments.
Guard the configured APIs with NET, leaving the netstandard2.0 API unchanged.
Update the inspected BugFinding and Rewriting IL-diff baselines. The other three
hashes remain unchanged.

This does not address CancellationTokenSource.CancelAsync interception or TPL
Dataflow scheduling support. ToBlockingEnumerable<T> retains native blocking
behavior; the forwarding overload fixes type compatibility, not scheduler semantics.

Validation

Windows, pinned .NET SDK 8.0.425:

Complete Release solution rebuild, including netstandard2.0: 0 warnings,
0 errors.
All six CI test suites (runtime, rewriting, testing, actors,
actors-testing, tools): 1,398 passed, 0 failed, 0 skipped.
ILVerify 8.0.0, matching CI: all classes and methods verified in all six
CI test assemblies.
Tests/compare-rewriting-diff-logs.ps1: all five hashes match.
git diff --check: clean.

Before the delegate fix, local ILVerify reproduced the previous CI run's
DelegateCtor and StackUnexpected failures in
TestConfigurationMethodGroup and TestCustomConfigurationMethodGroup.
After the fix, the complete BugFinding assembly verifies.

The six initial follow-up review reproductions exposed another 28 ILVerify
errors in bound delegates, boxing/unboxing, and array operations. Those are
fixed; two additional regressions verify nonvirtual struct binding and preserve
native instance binding for methods normally rewritten through static adapters.

Further inheritance/signature regressions reproduced six ILVerify errors
before the metadata correction. Verification also caught the base-constructor
call mismatch even while the runtime cases passed; the final fix verifies cleanly.

The dispatch/constraint regressions reproduced four runtime failures and five ILVerify
errors for explicit-interface overrides, recursive virtual base dispatch, and
type/method constraints. All four now pass, and all six assemblies verify.

The tuple/static-field regressions passed at runtime but reproduced
11 ILVerify errors before the field-reference correction; both now verify.
The null-source review claim was checked against the native CoreLib APIs by
reflection, bypassing rewriting: configuration accepts null, and enumeration
throws NullReferenceException. Three controls prove the wrappers preserve
that behavior; no incompatible immediate null guards were added.

Three array/external-container regressions reproduced 29 ILVerify errors, despite passing at
runtime, for rectangular-array constructors/accessors and unrewritten generic
containers/base classes. The final references verify, including a generic
method on a generic external container; all six assemblies verify cleanly.

The expression-token and blocking-enumeration cases reproduced five
runtime failures and four ILVerify errors before correction. Six cases now
pass, including a native instance-binding control, cancellation-token propagation,
and a previously mapped TaskAwaiter<int> element type.

The latest property/indexer and generic static-member expression regressions
reproduced four runtime failures before correction. All four now construct,
compile and execute successfully; an event metadata/dispatch control also passes.
Native factory identity is checked through reflection so the control does not
compare against a separately rewritten direct getter.

The six suites were invoked explicitly with dotnet vstest to avoid treating
the non-test Tests.Common helper assembly as a seventh test project.

GitHub Actions run 40399749907
passed on aa1a6f58: Coyote build/tests and sample build/tests on Windows,
Ubuntu, and macOS, plus CLA. All fifteen confirmed findings have verified
reproductions and fixes, and all eleven inline threads are resolved.
The latest automated review reports no open issues and recommends final human
review of the shared IL and metadata changes. The null-source false positive is
documented with native parity controls. Maintainer approval and merge remain.

Rewrite configured async-enumeration producers alongside their ValueTask consumers to avoid incompatible awaitable layouts. Cover cancellation, cleanup, context configuration, and failure replay. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3447f3e5-9328-4334-8f18-0223764709a5

Restore merge-only constructor regressions after rebasing onto main. Rewrite static method-group targets and bodyless custom delegate signatures consistently with configured awaitable types. Cover non-generic disposal delegates and update the inspected BugFinding IL baseline. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Initialize the shared type visitor state before processing bodyless delegate signatures. Fresh rebuild, focused regressions, ILVerify and all five IL-diff hashes remain clean. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Copilot

🟡 Changes recommended

Instance method-group targets and boxing operands can retain framework types incompatible with the rewritten Coyote types.

2 open issues

What changed in this PR

This PR fixes configured async enumeration and disposal rewriting by routing framework configuration APIs through Coyote wrappers and its controllable ValueTask awaitables.

Changes:

Add mappings and wrappers for configured enumerables, enumerators, and async disposables under NET.
Extend rewriting to static method-group targets and custom delegate signatures.
Add regression coverage and update the BugFinding IL-diff baseline.

File	Description
Tests/Tests.BugFinding/Tasks/ConfiguredAsyncEnumerableTests.cs	Adds enumeration, disposal, cancellation, constructor, and delegate regressions.
Tests/compare-rewriting-diff-logs.ps1	Updates the BugFinding rewrite hash.
Source/Test/Rewriting/Types/Threading/Tasks/TaskAsyncEnumerableExtensions.cs	Adds configuration entry points returning Coyote wrappers.
Source/Test/Rewriting/Types/Runtime/CompilerServices/ConfiguredCancelableAsyncEnumerable.cs	Wraps enumeration and disposal with controllable awaitables.
Source/Test/Rewriting/Types/Runtime/CompilerServices/ConfiguredAsyncDisposable.cs	Wraps configured async disposal.
Source/Test/Rewriting/Types/NameCache.cs	Registers framework configured-type names.
Source/Test/Rewriting/Passes/Rewriting/Types/TypeRewritingPass.cs	Maps configured framework types to Coyote replacements.
Source/Test/Rewriting/Passes/Rewriting/Types/MethodBodyTypeRewritingPass.cs	Rewrites static function-pointer targets while preserving `ldftn`.
Source/Test/Rewriting/Passes/Rewriting/Types/MemberTypeRewritingPass.cs	Includes custom delegate signatures in type rewriting.

_{Review effort: Balanced}

Frank Shearar (frankshearar) · 2026-10-26T17:58:06Z

Frank the Clank says: Pantazis Deligiannis (@pdeligia), this is rebased onto current main, with the already-merged constructor fix from #529 removed from the diff and its configured integration regressions preserved.

The previous CI failures were genuine delegate IL issues: static ldftn targets and bodyless custom-delegate signatures were not rewritten consistently. Both are corrected, with additional disposal-method-group coverage and an inspected IL baseline.

All checks on head 6c9cd31c966d1283a204d07ff9990dc1303f831a are green: Coyote and samples on Windows, Ubuntu, and macOS, plus CLA (run 40372631644). Local validation also passed all 1,364 CI-suite tests, ILVerify 8.0.0 for all six test assemblies, and all five IL-diff hashes.

Ready for maintainer review and merge. GitHub denied the author's formal reviewer request, so this comment is the review handoff.

Copilot

🟡 Changes recommended

Array allocation and access operands remain inconsistent with the newly mapped configured structs, producing invalid IL when those structs are stored in arrays.

3 open issues

_{Review effort: Balanced}

…ntly Remap all type operands, including boxing, unboxing and array access. Rewrite ldftn and ldvirtftn references without replacing instance binding with static adapters. Preserve rewritten caller generic arguments on external generic methods. Add eight regressions for bound methods, adapter controls, boxed round trips and arrays; refresh the inspected BugFinding IL baseline. All 1,372 tests and six ILVerify assemblies pass. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Copilot

🟡 Changes recommended

Unrewritten abstract/interface signatures and inheritance metadata can conflict with the new mappings, causing type-loading failures or invalid IL.

4 open issues

1 resolved since last review

Rewrite ldftn targets for remapped bound method groups

_{Review effort: Balanced}

Rewrite base and implemented-interface metadata. Visit all bodyless signatures during the member pass instead of special-casing delegates. Preserve instance targets for remapped base-constructor calls. Cover generic collection inheritance, explicit interface implementations, and abstract/interface parameter and return signatures. All 1,375 tests, six ILVerify assemblies and five hashes pass. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Copilot

🟡 Changes recommended

Remaining dispatch and generic-metadata inconsistencies can cause stack overflow, type-load failures, and constraint-validation failures.

3 open issues

5 resolved since last review

_{Review effort: Balanced}

Rewrite explicit interface declarations and generic constraints alongside method signatures. Preserve instance call opcodes so virtual base calls do not recurse and generic getters retain their original dispatch semantics. Add four regressions and refresh inspected IL baselines. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Copilot

🟡 Changes recommended

Generic field references can still produce incompatible IL, and enumerable configuration wrappers do not preserve null-source exception behavior.

1 open issue

Rewrite field declaring types consistently in tuple rewriting · New

1 previously missed issue

In code that hasn't changed since last review

Preserve immediate null-source exceptions in enumerable replacements

Source/Test/Rewriting/Types/Threading/Tasks/TaskAsyncEnumerableExtensions.cs:37

The enumerable ConfigureAwait and WithCancellation replacements accept a null source, whereas the framework APIs immediately throw ArgumentNullException("source"). Rewritten calls therefore succeed until enumeration throws NullReferenceException, changing exception handling during testing. Add immediate null-source checks to both methods (also at line 37).

3 resolved since last review

_{Review effort: Balanced}

Remap field declaring types as well as field types for all FieldReference operands. Cover tuple loads, stores and addresses plus generic static field access, reproducing eleven ILVerify errors before correction. Add native CoreLib controls confirming deferred null-enumerable exceptions without changing wrapper behavior. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Frank Shearar (frankshearar) · 2026-10-26T19:53:43Z

Frank the Clank says: The latest review overview also requests immediate ArgumentNullException("source") guards for enumerable ConfigureAwait and WithCancellation. That premise is incorrect for .NET 8: the framework extensions and configured enumerable constructor accept null, and GetAsyncEnumerator later throws NullReferenceException. In a6f6d98, TestNullEnumerableConfiguration checks all three configurations against the installed native CoreLib APIs via reflection, bypassing rewriting, then verifies identical rewritten behavior. All three controls pass. No immediate guards were added because they would change framework behavior. The separate generic-field issue is fixed with two regressions reproducing 11 ILVerify errors before the correction; all 1,384 local tests and all six verified assemblies now pass. New cross-platform CI is running.

Copilot

🔵 Needs a closer look

Shared IL and metadata changes require human review, with unresolved method-reference mismatches affecting external generic types and rectangular arrays.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Handle rectangular array constructor and accessor rewriting

Source/Test/Rewriting/Passes/Rewriting/Types/MethodBodyTypeRewritingPass.cs:71

Rectangular arrays such as new ConfiguredAsyncDisposable[1, 1] use synthetic constructor and accessor method references rather than the type operands handled here. Normal method rewriting cannot resolve those array methods, leaving framework-typed operands incompatible with rewritten locals and values. Add a dedicated array-method rewriting path that bypasses method-definition resolution for both allocation and element access, with a rectangular-array store/load regression.

Rewrite generic arguments on external types and base calls

Source/Test/Rewriting/Passes/Rewriting/Types/MethodBodyTypeRewritingPass.cs:179

For Box<T> in an unrewritten, non-System assembly, new Box<ConfiguredAsyncDisposable>(resource.ConfigureAwait(false)) retains the framework-typed constructor while its argument and local use Coyote's struct, producing incompatible IL. The same omission leaves base-constructor calls inconsistent with the remapped base type in MemberTypeRewritingPass.cs:33. Rewrite caller-supplied generic arguments on external declaring types as well as generic methods, preserving external concrete signatures, and add external generic-container and base-class regressions.

1 resolved since last review

Rewrite field declaring types consistently in tuple rewriting

_{Review effort: Balanced}

Rebuild synthetic array constructor/accessor references without resolving nonexistent method definitions. Remap caller-supplied arguments on external declaring types and generic methods without changing native concrete signatures. Add three regressions with an unrewritten generic helper, reproducing 29 ILVerify errors before correction, and refresh the inspected baseline. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Frank Shearar (frankshearar) · 2026-10-26T20:08:24Z

Frank the Clank says: Both additional findings in the latest review overview are reproduced and fixed in 9b4f1cd. TestConfiguredRectangularArrays covers allocation, Get, Set and Address for all three configured struct types; the rewriter now rebuilds synthetic array method references without definition lookup and preserves their opcodes. TestExternalConfiguredGenericContainer and TestExternalConfiguredGenericBase use an unrewritten Tests.Common helper to cover constructor arguments, getters, a generic method on a generic declaring type, inheritance and base-constructor calls. External caller-supplied type arguments are remapped while native concrete signatures remain untouched. All three cases passed at runtime before correction but exposed 29 ILVerify errors. After correction all 1,387 tests, six ILVerify assemblies and five inspected hashes pass, with zero build warnings/errors. Cross-platform CI for the new head is running.

Copilot

🔵 Needs a closer look

Broad IL and metadata transformations require human review, with unresolved method-token and generic-call compatibility issues.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Remap ldtoken method references in ConfigureAwait expression trees

Source/Test/Rewriting/Passes/Rewriting/Types/MethodBodyTypeRewritingPass.cs:83

Method-reference ldtoken operands are still skipped: Expression<Func<ConfiguredAsyncDisposable>> expression = () => source.ConfigureAwait(false) retains a token for the framework's ConfigureAwait. The rewritten delegate returns Coyote's struct while the expression-tree call returns the framework struct, so constructing the lambda throws ArgumentException. Remap method and constructor tokens while preserving their opcode and instance binding, and add an expression-tree regression.

Preserve ToBlockingEnumerable mapping for rewritten awaitable types

Source/Test/Rewriting/Passes/Rewriting/Types/TypeRewritingPass.cs:75

Mapping this class also redirects lookup of .NET 8's ToBlockingEnumerable<T>, but the replacement has no matching overload, so TryFindMethod fails before rewriting generic arguments. Calls with T = ConfiguredAsyncDisposable therefore retain the framework struct while rewritten producers and consumers use Coyote's struct, producing incompatible IL; already-mapped awaiter types are affected too. Add a framework-guarded forwarding overload, or retain the native target while rewriting its generic arguments, and add an IL-verification regression.

1 resolved since last review

Rewrite field declaring types consistently in tuple rewriting

_{Review effort: Balanced}

Remap method and constructor ldtoken operands while retaining opcodes and instance binding. Add a NET7_0_OR_GREATER native ToBlockingEnumerable forwarder so mapped generic arguments remain representation-consistent. Cover expression construction/compilation, native instance binding, cancellation propagation and previously mapped awaiters; reproduce five runtime failures and four ILVerify errors before correction. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Frank Shearar (frankshearar) · 2026-10-26T20:37:33Z

Frank the Clank says: Both new review-overview findings are reproduced and fixed in 777edc9. Method/constructor ldtoken operands now use method-reference rewriting while retaining ldtoken and instance binding; expression-tree construction/compilation tests cover configured disposal and a generic constructor, with native Task.Wait as an instance-binding control. The NET7_0_OR_GREATER ToBlockingEnumerable forwarding overload preserves mapped caller type arguments and cancellation while retaining native blocking behavior, not adding scheduler semantics. Blocking cases cover ConfiguredAsyncDisposable and the previously mapped TaskAwaiter, including cancellation-token propagation. Before correction these six cases produced five runtime failures and four ILVerify errors; after correction all 1,393 tests, six ILVerify assemblies and five inspected hashes pass with zero build warnings/errors. Cross-platform CI for the new head is running.

Copilot

🔵 Needs a closer look

Cross-cutting IL and metadata changes require human review, with unresolved metadata and method-handle inconsistencies affecting expression construction.

2 open issues

_{Review effort: Balanced}

Remap property, indexer and event metadata with accessor signatures. Preserve native generic declaring types for method tokens whose replacements are static adapters so paired method/type handles remain compatible. Add expression and event controls, and update the inspected IL diff hash. Copilot-Session: 240e36d3-c67f-4128-b182-3c10c4ea6879

Copilot

🔵 Needs a closer look

The cross-cutting changes to shared IL generation and metadata binding warrant final human review despite extensive regression coverage and reported validation.

No open issues 🎉

2 resolved since last review

_{Review effort: Balanced}

This was referenced Oct 22, 2026

Preserve newobj when rewritten targets remain instance constructors #528

Closed

Preserve newobj for rewritten instance constructors #529

Merged

Frank Shearar (frankshearar) and others added 2 commits October 26, 2026 10:34

Frank Shearar (frankshearar) force-pushed the fix/configured-async-enumeration branch from 611177f to 68c91eb Compare October 26, 2026 17:49

Copilot AI balanced review requested due to automatic review settings October 26, 2026 17:49

Copilot started reviewing on behalf of Frank Shearar (frankshearar) October 26, 2026 17:50

View session